Fix: Cosmos ToList on structural collection is treated as scalar#38122
Conversation
JoasE
changed the title
There was a problem hiding this comment.
I think these changes should be temporary, and #34067 will revert them.
I'm not as familiar with the projection binding code as other area's, but FWIG, this is the best solution right now
A bit of extra explanation per test case:
| """ | ||
| SELECT VALUE o["Details"] | ||
| SELECT VALUE o | ||
| FROM root c |
There was a problem hiding this comment.
This projection now happens on the client side in the materializer. This used to generate a JsonConvert.Deserialize call for materialization
There was a problem hiding this comment.
Because this is a SelectMany, we do bind the c["Orders"], but the "Details" projection part happens in the materializer
| public override Task Json_collection_leaf_filter_in_projection(bool async) | ||
| => Fixture.NoSyncTest( | ||
| async, async a => | ||
| { |
There was a problem hiding this comment.
This used to generate a JsonConvert.Deserialize call for materialization. I think we can't translate this because the Where would have to be ran in the materializer since we don't bind ["OwnedReferenceRoot"]["OwnedReferenceBranch"]["OwnedCollectionLeaf"], and 34067 should solve that
| public override Task Distinct() | ||
| => AssertTranslationFailed(base.Distinct); | ||
|
|
||
| public override async Task Distinct_projected(QueryTrackingBehavior queryTrackingBehavior) |
There was a problem hiding this comment.
This used to generate a JsonConvert.Deserialize call for materialization. I think we can't translate this because the Distinct would have to be ran in the materializer since we don't bind ["AssociateCollection"], and 34067 should solve that
JoasE
changed the title
JoasE
changed the title
There was a problem hiding this comment.
Pull request overview
Fixes Cosmos query projection binding for ToList() over structural (non-scalar) collections by avoiding scalar projection binding for non-scalar subquery results.
Changes:
- Adjust Cosmos projection binding so
ToList()only binds scalar array subquery results as a scalar projection; non-scalar results are visited/shaped instead. - Update Cosmos functional test SQL baselines for owned structural collection
ToList()projections. - Update/realign Cosmos tests to assert translation failure for scenarios that were previously (incorrectly) translated for structural collections (e.g.
Where/Distinctcomposed beforeToList()).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/EFCore.Cosmos.FunctionalTests/Query/OwnedQueryCosmosTest.cs | Updates expected Cosmos SQL to project the owning object (o) rather than o["Details"] for structural collection ToList() projections. |
| test/EFCore.Cosmos.FunctionalTests/Query/JsonQueryCosmosTest.cs | Changes a JSON structural collection filter-in-projection test to assert translation failure instead of asserting generated SQL. |
| test/EFCore.Cosmos.FunctionalTests/Query/Associations/ComplexProperties/ComplexPropertiesCollectionCosmosTest.cs | Changes Distinct_projected to assert translation failure rather than asserting generated SQL. |
| src/EFCore.Cosmos/Query/Internal/CosmosProjectionBindingExpressionVisitor.cs | Prevents binding non-scalar array subquery results (e.g. structural/object arrays) as scalar projections when translating ToList(); adds a translation-failure throw for unexpected Queryable methods in this client-eval path. |
|
@JoasE Please rebase on main |
… projection Don't bind non scalar subquery results for ToList projections
AndriySvyryd
left a comment
AndriySvyryd
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
Don't bind non scalar subquery results for ToList projections
This would change behaviour from 10.0.0, where some queries compiled and might have worked, but actually didn't use a real materializer. If the user didn't have any custom ef config and their model matches the json
Closes: #38121